Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway: expose commands for js-libp2p delegated routing #4595

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 19, 2018

This enables js-libp2p/js-ipfs to delegate peer routing and content routing to a go-ipfs gateway (usually ipfs.io). It covers:

  • Peer routing lookups (dht/findpeer)
  • Content routing lookups (dht/findprovs)
  • Content routing writes -- go-libp2p-kad-dht rejects provider records which originate from a different node than the message sender, so we're doing a creative hack here: we'll tell the delegatee to connect to us (swarm/connect with a /p2p-circuit address), and then to fetch the data from us (refs?r=true). This isn't technically content routing, but it makes the data available to the network which is all we need.

Peer routing writes will be a new command that'll be covered in a future PR.

(@diasdavid catch!)

@ghost ghost added topic/libp2p Topic libp2p topic/commands Topic commands topic/gateway Topic gateway topic/interop Interoperability labels Jan 19, 2018
@ghost ghost self-assigned this Jan 19, 2018
@ghost ghost added the status/in-progress In progress label Jan 19, 2018
@ghost
Copy link
Author

ghost commented Jan 19, 2018

Oh and this is already running on ipfs.io, so go take a shot at it @diasdavid

@Stebalien
Copy link
Member

Would putting these behind a config option be problematic? I worry about increasing the default gateway API a bit.

@ghost
Copy link
Author

ghost commented Jan 28, 2018

Would putting these behind a config option be problematic? I worry about increasing the default gateway API a bit.

Not at all, you have a point. I'll update this in the next few days.

@Kubuxu
Copy link
Member

Kubuxu commented Mar 9, 2018

@magik6k mind taking over?

@Kubuxu Kubuxu added need/author-input Needs input from the original author and removed status/in-progress In progress labels Mar 9, 2018
@magik6k magik6k self-assigned this Mar 9, 2018
@magik6k
Copy link
Member

magik6k commented Mar 9, 2018

I don't see any reasonably simple way to put this behind a config option (rootROOldSubcommands is set up in init), one 'hack' I have in mind is to create a utility command which would perform that check and proxy to the proper command, does that sound reasonable?

@Stebalien
Copy link
Member

Can the command itself not check and return a permission denied error? Not the best solution but it works.

@magik6k
Copy link
Member

magik6k commented Mar 9, 2018

It would have to check if it was called from the gateway port in that case which I'm not sure is even possible.

@ghost
Copy link
Author

ghost commented Mar 24, 2018

We could maybe eliminate the root/rootRO duality and have :8080 call the same root cmds.Command as :5001, but with a configurable allowlist in front. The current rootRO would then be default config for that.

@ghost
Copy link
Author

ghost commented Mar 24, 2018

but with a configurable allowlist in front

and a hardcoded list of commands that should not ever be exposed to the public.

@ghost ghost force-pushed the feat/delegated-routing branch from 514af5e to 0dc0a04 Compare April 18, 2018 17:04
@ghost ghost requested a review from Kubuxu as a code owner April 18, 2018 17:04
@ghost ghost added the status/in-progress In progress label Apr 18, 2018
@ghost ghost force-pushed the feat/delegated-routing branch from 0dc0a04 to 287cbed Compare April 21, 2018 08:59
ghost pushed a commit to ipfs/infra that referenced this pull request Apr 23, 2018
Exposes API methods for delegated routing on the gateway.

License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
ghost pushed a commit to ipfs/infra that referenced this pull request Apr 23, 2018
@ghost ghost force-pushed the feat/delegated-routing branch from 287cbed to 6c76de3 Compare August 22, 2018 15:54
@ghost
Copy link
Author

ghost commented Aug 28, 2018

I'm picking this back up this week -- let me know if there's reservations about the approach mentioned back in March:

We could maybe eliminate the root/rootRO duality and have :8080 call the same root cmds.Command as :5001, but with a configurable allowlist in front. The current rootRO would then be default config for that.

License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@ghost ghost force-pushed the feat/delegated-routing branch from 6c76de3 to 23a68cd Compare August 31, 2018 13:32
@jacobheun
Copy link
Contributor

@lgierth I think we may need to add some limiters on findProviders. I am testing this locally with an example of the current delegated routing implementation in js-libp2p. Other than limiting the number of providers there isn't currently any other limiter to the call. If I run ipfs dht findprovs for a newer CID and the number of providers is below 20 (the default), or the query takes a while to find 20, the connection stays open until at least 20 providers can be found.

For an http request to the delegated node, this can mean multiple http requests open for minutes at a time. For content I just added to my local go-ipfs instance, I've seen it exceed 30minutes for an open connection in browser.

@ghost
Copy link
Author

ghost commented Sep 4, 2018

@jacobheun In go-ipfs there's a -n/--num-providers (default=20) option on ipfs dht findprovs. But nothing similar on ipfs dht findpeer yet.

@ghost
Copy link
Author

ghost commented Sep 12, 2018

@jacobheun Just to confirm, do you need limiting on findprovs too?

@jacobheun
Copy link
Contributor

@lgierth my primary concern is too many open connections to the server. The current code for findprovs keeps the connection open until the default 20 providers are returned.

In the event that findprovs is called for new content by several peers, the requests from those peers will stay open until resolved (which can take a significant amount of time).

Even though I can lower --num-providers, it's not the default so most people won't use it, unless we explicitly lower it in the js module.

I'm wondering if a default timeout is needed for these requests?

@ghost
Copy link
Author

ghost commented Sep 14, 2018

Yeah sounds like some combination of --num-providers and --timeout is needed, and a reasonable default should be used in the delegated routing module.

@ghost
Copy link
Author

ghost commented Sep 14, 2018

@jacobheun Just to confirm, do you need limiting on findprovs too?

Eeeh what I meant to say here: do you need the limiting on findpeer too?

@jacobheun
Copy link
Contributor

@lgierth I think --timeout limiting for findpeer would be great.

@ghost
Copy link
Author

ghost commented Sep 14, 2018

Okay, cool -- --timeout already exists in go-ipfs, it's an option on the root command: ipfs --help

@jacobheun
Copy link
Contributor

Perfect, I can work on adding support and sane defaults for those in the routing modules.

@jacobheun
Copy link
Contributor

I've got the js-libp2p delegated routing work all ready to go and in review. I've got the timeouts configurable and currently defaulting to a max of 30 seconds. It's been verified against the example I demo'd a few weeks back, which is also in the PR for js-libp2p libp2p/js-libp2p#242

@ghost
Copy link
Author

ghost commented Sep 27, 2018

Update on the go side is that configurable Gateway API commands (aka "readonly API" commands) are ready, but I'm dealing with some nasty dependency issues - hopefully done by tomorrow.

@ghost ghost mentioned this pull request Oct 5, 2018
@ghost
Copy link
Author

ghost commented Oct 5, 2018

Closing this in favor of #5565

@ghost ghost closed this Oct 5, 2018
@ghost ghost removed the status/in-progress In progress label Oct 5, 2018
@ghost ghost deleted the feat/delegated-routing branch October 5, 2018 02:52
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/author-input Needs input from the original author topic/commands Topic commands topic/gateway Topic gateway topic/interop Interoperability topic/libp2p Topic libp2p
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants